Skip to content

Comments

feat: add keycloak with minecraft extension#49

Merged
lusu007 merged 7 commits intomainfrom
feat/keycloak
Feb 23, 2026
Merged

feat: add keycloak with minecraft extension#49
lusu007 merged 7 commits intomainfrom
feat/keycloak

Conversation

@hbrombeer
Copy link
Member

@hbrombeer hbrombeer commented Feb 23, 2026

Pull Request

Description

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 💥 Breaking change
  • ♻️ Refactoring
  • 📚 Documentation
  • 🔧 Chore

Related Issues

  • Fixes #

Testing

  • Unit tests pass
  • Manual testing completed
  • New tests added for new functionality

Checklist

  • I have performed a self-review of my own code
  • Tests have been added/updated and pass (if needed)
  • Documentation has been updated (if needed)

Note

Medium Risk
Introduces a new production identity-provider image build that pulls an external JAR at build time and adds it to automated publish/release pipelines, so supply-chain and runtime behavior need review.

Overview
Adds a new keycloak container image based on Keycloak 26.1 that downloads and bundles the keycloak-minecraft provider during a builder stage, then runs with start --optimized and OCI metadata labels.

Integrates keycloak into repo automation by adding it to CI/build-and-push Docker matrices, enabling Dependabot Docker updates for keycloak/, and registering it with release-please (manifest + config) plus top-level README docs.

Written by Cursor Bugbot for commit 4ca5930. Configure here.

@hbrombeer hbrombeer requested a review from lusu007 February 23, 2026 18:19
@CLAassistant
Copy link

CLAassistant commented Feb 23, 2026

CLA assistant check
All committers have signed the CLA.

@hbrombeer
Copy link
Member Author

@claude

@claude
Copy link

claude bot commented Feb 23, 2026

Claude finished @hbrombeer's task in 1m 36s —— View job


Code Review

Overall the PR is well-structured and follows the repo's established patterns. A few issues are worth addressing before merge.


keycloak/Dockerfile — Issues

1. No checksum verification for the downloaded JAR (medium severity)

keycloak/Dockerfile:7:

ADD --chmod=644 "https://github.com/groundsgg/keycloak-minecraft/releases/download/v${KEYCLOAK_MINECRAFT_VERSION}/keycloak-minecraft.jar" /opt/keycloak/providers/keycloak-minecraft.jar

There is no SHA-256 or signature verification after downloading the JAR. If the release artifact were replaced (even accidentally), the build would silently include it. Since this is your own org's repo the risk is lower, but adding a digest check would harden the supply chain. Consider adding a --checksum option (Docker BuildKit supports ADD --checksum=sha256:<hash> <url>) or a RUN sha256sum -c step. Fix this →

2. KEYCLOAK_MINECRAFT_VERSION ARG won't be auto-updated by Dependabot

keycloak/Dockerfile:3:

ARG KEYCLOAK_MINECRAFT_VERSION=1.0.0

Dependabot's Docker ecosystem monitor tracks base image tags, not ARG defaults. This means the extension version will never receive automatic PRs. The base Keycloak image will be kept up to date, but the keycloak-minecraft JAR version will not. Worth documenting this or tracking it manually.

3. Missing EXPOSE directive

Keycloak listens on ports 8080 (HTTP) and 8443 (HTTPS) by default, but neither is declared. This is a minor gap — compare with paper/Dockerfile:46 and velocity/Dockerfile:48 which both EXPOSE 25565. Fix this →

4. Missing HEALTHCHECK directive

Both paper/Dockerfile:48-49 and velocity/Dockerfile:50-51 include a HEALTHCHECK. The Keycloak image omits one. Keycloak exposes a health endpoint at /health (or /health/ready) on port 8080 which would be appropriate here. Fix this →


CI/CD Changes — OK

.github/workflows/ci.yml and docker-build-push.yml each add keycloak to the matrix in one line. These are correct and consistent with how paper and velocity are registered.


Dependabot — OK

The new entry for /keycloak is consistent with the other Docker entries. One note: as mentioned above, this will track the quay.io/keycloak/keycloak base image tag but not the KEYCLOAK_MINECRAFT_VERSION ARG.


Release-please — OK

Starting at 0.1.0 is appropriate. The release-please-config.json entry is consistent with other packages.


keycloak/README.md — Minor

The README is functional but thinner than other container READMEs. It's missing:

  • Registry pull command (e.g. docker pull ghcr.io/groundsgg/containers/keycloak)
  • Required environment variables section (at minimum KC_HOSTNAME, KC_DB* are required to run — currently they're listed as "common configuration" but not flagged as required)

Summary

Severity Issue
Medium No checksum verification for downloaded JAR
Low Missing EXPOSE 8080 8443
Low Missing HEALTHCHECK
Info KEYCLOAK_MINECRAFT_VERSION not tracked by Dependabot
Info README could be more complete

The CI/build pipeline integration, release-please config, and dependabot setup are all done correctly.

@lusu007
Copy link
Contributor

lusu007 commented Feb 23, 2026

Regarding Claudes comment:

We should only implement

  • EXPOSE 8080 8443
  • a HEALTHCHECK using the Keycloak readiness endpoint
  • a documentation note clarifying that KEYCLOAK_MINECRAFT_VERSION is not tracked by Dependabot
  • the missing pull command and required environment variables in the README

Regarding the checksum verification: since the JAR is downloaded from our own GitHub org and versioned via an explicit release tag, I’m comfortable with the current setup and would prefer not to add additional checksum handling at this stage.

hbrombeer and others added 2 commits February 23, 2026 19:57
- Use /health/ready endpoint for HEALTHCHECK instead of plain TCP check
- Enable health endpoint during kc.sh build
- Add pull command and dependency update note to README
- Mark environment variables as required

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hbrombeer hbrombeer requested a review from lusu007 February 23, 2026 19:02
@hbrombeer hbrombeer requested a review from lusu007 February 23, 2026 19:08
@lusu007 lusu007 changed the title add keycloak with minecraft extension feat: add keycloak with minecraft extension Feb 23, 2026
@lusu007 lusu007 merged commit ac325b2 into main Feb 23, 2026
7 checks passed
@lusu007 lusu007 deleted the feat/keycloak branch February 23, 2026 19:18
lusu007 pushed a commit that referenced this pull request Feb 23, 2026
* add keycloak with minecraft extension

* changed from curl to native ADD

* feat(keycloak): add healthcheck, expose ports, and clean up README

* fix(keycloak): address PR review feedback

- Use /health/ready endpoint for HEALTHCHECK instead of plain TCP check
- Enable health endpoint during kc.sh build
- Add pull command and dependency update note to README
- Mark environment variables as required

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* bumped version to 26.5.0

* removed renovate & bumped to 26.5.4

* removed changelog.md

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
lusu007 added a commit that referenced this pull request Feb 23, 2026
* feat: add keycloak with minecraft extension (#49)

* add keycloak with minecraft extension

* changed from curl to native ADD

* feat(keycloak): add healthcheck, expose ports, and clean up README

* fix(keycloak): address PR review feedback

- Use /health/ready endpoint for HEALTHCHECK instead of plain TCP check
- Enable health endpoint during kc.sh build
- Add pull command and dependency update note to README
- Mark environment variables as required

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* bumped version to 26.5.0

* removed renovate & bumped to 26.5.4

* removed changelog.md

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(deps): bump actions/checkout from 4 to 6

Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 6.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v4...v6)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Hendrik Brombeer <20863417+hbrombeer@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Lukas Jost <lukas.jost@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants